feat(kinesis): Kinesis Data Streams Library#6516
Conversation
packages/kinesis/aws_kinesis_datastreams/lib/src/impl/record_storage.dart
Outdated
Show resolved
Hide resolved
packages/kinesis/aws_kinesis_datastreams/lib/src/impl/kinesis_sender.dart
Outdated
Show resolved
Hide resolved
packages/kinesis/aws_kinesis_datastreams/lib/src/exception/kinesis_exception.dart
Outdated
Show resolved
Hide resolved
| final Future<void> Function() _onFlush; | ||
| Timer? _timer; | ||
| bool _enabled = true; | ||
| bool _closed = false; |
There was a problem hiding this comment.
What is the benefit of having both _enabled and _closed?
There was a problem hiding this comment.
_closed can serve as a flag for other code to know whether or not the client itself is shut down or if just flushing is enabled or not. In our implementation they do similar things but it is useful for getters to know the internal state of the client I feel. What do you think?
There was a problem hiding this comment.
I think both approaches are fine, but personally, I recommend merging them for the auto flush scheduler, since _closed is not really used.
packages/kinesis/aws_kinesis_datastreams_dart/lib/src/impl/kinesis_sender.dart
Outdated
Show resolved
Hide resolved
packages/kinesis/aws_kinesis_datastreams/test/e2e/kinesis_e2e_test.dart
Outdated
Show resolved
Hide resolved
packages/kinesis/aws_kinesis_datastreams/test/e2e/kinesis_e2e_test.dart
Outdated
Show resolved
Hide resolved
packages/kinesis/aws_kinesis_datastreams_dart/test/e2e/kinesis_e2e_test.dart
Outdated
Show resolved
Hide resolved
packages/kinesis/aws_kinesis_datastreams/test/e2e/kinesis_e2e_test.dart
Outdated
Show resolved
Hide resolved
| }); | ||
| }); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Can we ensure concurrent access to the record storage works via a test?
a21d70d to
ff90989
Compare
d9ec047 to
8ec0ec5
Compare
packages/kinesis/aws_kinesis_datastreams_dart/lib/src/db/kinesis_record_database.dart
Outdated
Show resolved
Hide resolved
| /// {@template aws_kinesis_datastreams.kinesis_service_exception} | ||
| /// Thrown when a Kinesis SDK/API error occurs. Inspect [sdkException] for details. | ||
| /// {@endtemplate} | ||
| final class KinesisServiceException extends AmplifyKinesisException { |
There was a problem hiding this comment.
I think we discussed that those errors are silently swallowed and not exposed.
| /// Thrown when a single record exceeds the Kinesis per-record size limit | ||
| /// (10 MiB, partition key + data blob combined). | ||
| /// {@endtemplate} | ||
| final class KinesisRecordTooLargeException extends AmplifyKinesisException { |
There was a problem hiding this comment.
If we have a specific type for records being too large, we should also have one for the partition key being too long.
There was a problem hiding this comment.
agreed, added a corresponding type
| if (_closed) throw ClientClosedException(); | ||
| if (!_enabled) return; | ||
|
|
||
| if (record.dataSize > kKinesisMaxRecordBytes) { |
There was a problem hiding this comment.
We should also check the partition key length here. It should be in (0, 256]
packages/kinesis/aws_kinesis_datastreams/lib/src/impl/record_client.dart
Outdated
Show resolved
Hide resolved
| ''', | ||
| variables: [Variable.withInt(maxCount), Variable.withInt(maxBytes)], | ||
| readsFrom: {_db.kinesisRecords}, | ||
| ).get(); |
There was a problem hiding this comment.
What happens if this query fails? In my understanding, currently, we'd swallow the error in flush's try-catch block. However, I think we would like to expose such errors as storage errors. Can we add logic to expose such errors as Kinesis* errors? Or am I missing something?
There was a problem hiding this comment.
no, good call, wrapped the failures in error object
|
|
||
| /// Returns the current total size of cached data in bytes. | ||
| Future<int> getCurrentCacheSize() async { | ||
| final query = _db.selectOnly(_db.kinesisRecords) |
There was a problem hiding this comment.
I think, I commented this before: We can keep an integer for the cache size and update / check against that, when we add new records. We will only need to run a DB query after removing records. This allows us to avoid the extra query when recording, which is most likely the most frequent operation.
5ffac53 to
11bb6e0
Compare
Adds the foundational package with minimal constructs needed for the Kinesis client libraries: - AmplifyException base class - AWSCredentialsProvider<T> and AWSCredentials sealed hierarchy - Logger interface with AmplifyLogger implementation - LogLevel enum - Result<T, E> sealed type
Provides V2CredentialsProviderBridge to bridge aws_common (V2) credentials to amplify_foundation_dart (V3) credentials. Shared by kinesis client packages to avoid duplicating bridge logic.
* feat(kinesis): return RecordData from record() for cross-platform alignment\n\nChange record() return type from Result<void> to Result<RecordData>\nacross both the Dart client and Flutter wrapper. RecordData contains\nthe recorded entry size (data + partition key bytes), aligning with\nthe Android and Swift Kinesis client APIs.\n\nWhen the client is disabled, returns RecordData(recordSize: 0)." * feat(kinesis): return RecordData from record() for cross-platform alignment\n\nChange record() return type from Result<void> to Result<RecordData>\nacross both the Dart client and Flutter wrapper. RecordData contains\nthe recorded entry size (data + partition key bytes), aligning with\nthe Android and Swift Kinesis client APIs.\n\nWhen the client is disabled, returns RecordData(recordSize: 0)."
refactor(kinesis): rename kinesis_data_streams_options.dart to amplify_kinesis_client_options.dart Align the options file name with the client class name for consistency. Co-authored-by: jvh-aws <jvhoff@amazon.de>
* Reapply "Align exceptions with Amplify v2 AmplifyException pattern\n\nSwitch AmplifyKinesisException to extend amplify_core's\nAmplifyException instead of amplify_foundation_dart's.\nAdopt positional message, optional recoverySuggestion,\nunderlyingException (was cause), const constructors,\nand runtimeTypeName overrides."" This reverts commit f660c5f. * Add const to ClientClosedException() call sites --------- Co-authored-by: jvh-aws <jvhoff@amazon.de>
Expose library version in foundation, use it in kinesis user agent
* Expose library version in foundation, use it in kinesis user agent * Fix flush strategy naming * Apply autoformat * Apply autoformat
* Expose library version in foundation, use it in kinesis user agent * Fix flush strategy naming * Apply autoformat * Apply autoformat * Rename package from aws_kinesis_datastreams to amplify_kinesis * Fix analyzer
This reverts commit c3cfb6c.
Strip RecordData to an empty wrapper for now. Fields can be added later once cross-platform alignment on the return shape is finalized.
* Minor doc fixes and remove reexport * Adapt public facing docstrings
…6795) * refactor: remove Category.kinesis enum, use string categories in deploy script Decouple deploy_gen2.dart from the amplify_core Category enum by changing AmplifyBackendGroup.category to a plain String. This allows removing Category.kinesis from the enum and its dead switch case in AmplifyClassImpl, since the kinesis client is standalone and does not participate in the Amplify plugin registration system. The --category CLI flag now derives allowed values from infraConfig instead of Category.values. * fix: match original Category.api.name casing ('API' not 'Api')
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.